Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Edit DVC deps to include cmd run files #45

Merged
merged 6 commits into from
Dec 16, 2024
Merged

Edit DVC deps to include cmd run files #45

merged 6 commits into from
Dec 16, 2024

Conversation

wagnerlmichael
Copy link
Member

This PR edits the dvc.yaml file's deps to be consistent with the documentation by adding the files invoked as deps.

@wagnerlmichael wagnerlmichael linked an issue May 13, 2024 that may be closed by this pull request
Edit dvc.yaml to include files invoked as deps #44
Closed
@wagnerlmichael wagnerlmichael marked this pull request as ready for review May 13, 2024 14:50
dfsnow
dfsnow requested changes May 13, 2024
View reviewed changes
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same deal for this PR @wagnerlmichael. Let's add the hashed files to the lock file.

@wagnerlmichael
Copy link
Member Author

@dfsnow mentioned I should update the hash of the dvc.lock file since I added the files themselves in the pipeline stage for the dvc.yaml pipeline. Currently there are only 6 stages in the dvc.lock file although there are 7 total stages in the dvc.yaml. Should we update the dvc.lock to incorporate all 7 stages? This is the only way it makes sense for me to add hash data @wrridgeway

@wagnerlmichael
Copy link
Member Author

Maybe it isn't super necessary to have all 7 stages included. I'll ping for review to get the ball rolling.

@wagnerlmichael wagnerlmichael requested a review from dfsnow August 14, 2024 20:43
dfsnow
dfsnow reviewed Aug 14, 2024
View reviewed changes
Copy link
Member

@dfsnow dfsnow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wagnerlmichael I just merged master into this branch. Can you pull and rehash the deps to catch any changes from the code that got pulled in? Then this should be good to go.

@wagnerlmichael
Copy link
Member Author

I actually do think we should run dvc again and hook up the missing file in dvc.lock. Does that sound right? @dfsnow

@dfsnow
Copy link
Member

dfsnow commented Aug 17, 2024

I actually do think we should run dvc again and hook up the missing file in dvc.lock. Does that sound right? @dfsnow

Yes please @wagnerlmichael

@dfsnow dfsnow changed the base branch from master to 2025-assessment-year December 16, 2024 16:14
@dfsnow dfsnow merged commit 63b0008 into 2025-assessment-year Dec 16, 2024
3 checks passed
@dfsnow dfsnow deleted the 44-edit-dvcyaml-to-include-files-invoked-as-deps branch December 16, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dfsnow dfsnow dfsnow requested changes

@wrridgeway wrridgeway Awaiting requested review from wrridgeway wrridgeway is a code owner

@jeancochrane jeancochrane Awaiting requested review from jeancochrane jeancochrane is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Edit dvc.yaml to include files invoked as deps
2 participants
@wagnerlmichael @dfsnow